Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TKNECO-93: Buildah Task Documentation #39

Closed
wants to merge 4 commits into from

Conversation

Senjuti256
Copy link
Contributor

@Senjuti256 Senjuti256 commented Aug 31, 2023

Basic documentation as per story: https://issues.redhat.com/browse/TKNECO-92

Drafted a preliminary documentation for users to get a hold of the Buildah Task.

@Senjuti256 Senjuti256 added documentation Improvements or additions to documentation buildah labels Sep 4, 2023
Copy link
Contributor

@otaviof otaviof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good write up, thank you! Please, align the modifications following #40, and consider the review comments as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sync up with @Aneesh-M-Bhat's PR #40, check if there are any differences from what you're doing with his and merge into a single place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

doc/buildah.md Outdated
Comment on lines 1 to 4
Containers Tekton Tasks
-----------------------

# `Buildah` Tekton Task
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please the following snippet for the page tittle:

Buildah Tekton Task
-------------------

doc/buildah.md Outdated
Comment on lines 32 to 41
In case the Container Registry requires authentication, please consider the [Tekton Pipelines documentation][tektonPipelineAuth]. In a nutshell, you need to create a Kubernetes Secret describing the following attributes:

```bash
kubectl create secret docker-registry imagestreams \
--docker-server="image-registry.openshift-image-registry.svc:5000" \
--docker-username="${REGISTRY_USERNAME}" \
--docker-password="${REGISTRY_TOKEN}"
```

Then make sure the Secret is linked with the Service-Account running the `TaskRun`/`PipelineRun`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the previous PR (#40) we end up repeating this part, let's please extract the container registry authentication configuration into a single .md file, so we can link this and future documentation to the same place.

doc/buildah.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a docs folder, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have used it already.

doc/buildah.md Outdated
Comment on lines 22 to 25
- name: TLS_VERIFY
value: true
- name: VERBOSE
value: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a shorter example, you can omit the Params with default values, like these two.

doc/buildah.md Outdated
Comment on lines 53 to 54
| `BUILD_EXTRA_ARGS` | `string` | `""` | Extra parameters passed for the build command when building images. |
| `PUSH_EXTRA_ARGS` | `string` | `""` | Extra parameters passed for the push command when pushing images. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty should just be ``, instead of "".

doc/buildah.md Outdated

# `Buildah` Tekton Task

The `buildah` Task is meant to build [OCI][OCI] container images without the requirement of container runtime daemon like Docker daemon using [Buildah][containersBuildah], the Task results contain the image name and the SHA256 image digest.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extract this paragraph on a # Abstract document section.

doc/buildah.md Outdated

The `buildah` Task is meant to build [OCI][OCI] container images without the requirement of container runtime daemon like Docker daemon using [Buildah][containersBuildah], the Task results contain the image name and the SHA256 image digest.

Please, consider the usage example below:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And let's create a new section named # Usage, please.

doc/buildah.md Outdated

Then make sure the Secret is linked with the Service-Account running the `TaskRun`/`PipelineRun`.

## Params
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a new section for the Workspaces documentation.

@otaviof otaviof changed the title Documentation drafted for Buildah Task. TKNECO-93: Buildah Task Documentation Sep 4, 2023
@Senjuti256 Senjuti256 requested a review from otaviof September 6, 2023 04:39
@Senjuti256 Senjuti256 closed this Sep 6, 2023
@Senjuti256 Senjuti256 deleted the Documentation branch September 6, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buildah documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants